Skip to content

AreaMeshFactory ::combineメソッドでもgroup単位で分割するように変更#285

Open
azukizuki wants to merge 2 commits intodev/v4from
feature/tile_group
Open

AreaMeshFactory ::combineメソッドでもgroup単位で分割するように変更#285
azukizuki wants to merge 2 commits intodev/v4from
feature/tile_group

Conversation

@azukizuki
Copy link
Contributor

@azukizuki azukizuki commented Nov 1, 2025 •
edited by coderabbitai bot
Loading

🔗 関連リンク

https://synesthesias.atlassian.net/browse/CPSDK25-134

✅ レビュー前確認項目

  • 自動ビルド・テストが通っていること

✅ マージ前確認項目

  • (libcitygmlの変更がある場合)libcitygmlがmasterの最新版になっていること

🚀 実装内容

LOD1テクスチャを貼る対応でgroup単位でメッシュが分かれていないと撮影がうまくいかないためLib側に修正を入れました

🌐 影響範囲

🛠️ 動作確認

⚠️ 懸念点

Summary by CodeRabbit

リリースノート

  • Refactor
    • メッシュ生成をグループ処理の二段階フローに改め、処理効率と統合精度を向上しました。
    • LOD（詳細度）判定を強化して不要な重複メッシュを省き、出力品質とパフォーマンスを改善しました。
  • Bug Fixes
    • 複数オブジェクトの統合処理での不整合を解消し、最終的なメッシュ結合の安定性を高めました。

@coderabbitai
Copy link

coderabbitai bot commented Nov 1, 2025 •
edited
Loading

Walkthrough

シティオブジェクトの即時メッシュ生成を廃し、各オブジェクトの gmlPath を保持したまま (group_id, grid_id=0) でグループ化してから、グループごとに MeshFactory を作成してメッシュ生成・最適化・統合する二段階処理に変更。

Changes

Cohort / File(s) 変更の要約
メッシュ生成の二段階化と gmlPath 伝播
src/polygon_mesh/area_mesh_factory.cpp		 各 CityObject とその gmlPath をマップ化する GroupGridIDToObjectsWithPathMap を導入。オブジェクトを (group_id, grid_id=0) で集約し、各オブジェクトの最大 LOD を計算して highest_lod_only に基づきフィルタリング。グループごとに MeshFactory を生成し、プライマリ／アトミック（子）オブジェクトを gmlPath と共に追加してメッシュを生成・最適化後、グループメッシュを最終的にマージするように制御フローを変更。

Sequence Diagram(s)

sequenceDiagram
    participant Input as シティオブジェクト一覧
    participant Mapper as GroupMap作成
    participant Grouper as グループ化 (group_id, grid_id=0)
    participant PerGroup as グループ毎 MeshFactory
    participant Merger as GridMergeResult

    Note over Input,Mapper: 各 CityObject と gmlPath をペアで収集
    Input->>Mapper: CityObject + gmlPath
    Mapper->>Grouper: GroupGridIDToObjectsWithPathMap
    Note over Grouper: 各オブジェクトの max_lod を計算\nhighest_lod_only フィルタ適用
    Grouper->>PerGroup: 各グループのオブジェクト集合
    loop グループ毎処理
        PerGroup->>PerGroup: MeshFactory を生成
        PerGroup->>PerGroup: プライマリ + 子(LOD≥2) を追加 (gmlPath保持)
        PerGroup->>PerGroup: メッシュ生成 → 最適化
        PerGroup->>Merger: 最適化済みグループメッシュを追加
    end
    Merger->>Merger: グループ結果をマージして GridMergeResult を返す
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • 注目ポイント:
    • group_id の算出ロジックと max LOD の判定が元実装と整合しているか
    • gmlPath の伝搬が全てのメッシュ生成パスで正しく扱われているか
    • highest_lod_only フラグが期待通りに非対象を除外すること
    • グループ別 MeshFactory のライフサイクルとメモリ／スコープ管理
    • 最終マージでメッシュ重複やトポロジーの破綻が発生しないか

Poem

🐰 小さな足で道を描き、
gmlPath を手に集めるよ。
グループに分けて、丁寧に編み、
二段の舞でメッシュは整う。
ほら、ウサギの庭に新しい形が生える。

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning 関連リンク、レビュー前確認項目は記載されていますが、影響範囲、動作確認、懸念点といった重要なセクションが記載されていません。 影響範囲、動作確認手順と結果、懸念点を具体的に記載してください。特にメッシュ分割の影響範囲と動作確認方法が必須です。
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed PR タイトルはAreaMeshFactory::combine メソッドでのgroup単位分割という実装内容の主要な変更を正確に伝えており、ファイルサマリーと一致しています。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/tile_group

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Nov 1, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e7950e and 3f35536.

📒 Files selected for processing (1)
  • src/polygon_mesh/area_mesh_factory.cpp (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.941Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。
📚 Learning: 2025-08-18T07:10:22.941Z 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.941Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Applied to files:

  • src/polygon_mesh/area_mesh_factory.cpp
📚 Learning: 2025-08-19T07:27:43.111Z 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。

Applied to files:

  • src/polygon_mesh/area_mesh_factory.cpp
📚 Learning: 2025-08-19T07:27:43.111Z 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

Applied to files:

  • src/polygon_mesh/area_mesh_factory.cpp
🧬 Code graph analysis (1)
src/polygon_mesh/area_mesh_factory.cpp (2)
src/polygon_mesh/polygon_mesh_utils.cpp (4)
  • findFirstPolygon (83-97)
  • findFirstPolygon (83-83)
  • getChildCityObjectsRecursive (73-81)
  • getChildCityObjectsRecursive (73-73)
src/polygon_mesh/mesh_extractor.cpp (4)
  • isTypeToSkip (210-217)
  • isTypeToSkip (210-210)
  • shouldContainPrimaryMesh (198-208)
  • shouldContainPrimaryMesh (198-198)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: build-and-test (macos-14, arm64)
  • GitHub Check: upload-dlls (macos-14, arm64)
  • GitHub Check: upload_mobile_dlls (ubuntu-latest, 33, 30.0.3, 23.1.7779620)
  • GitHub Check: build-and-test (ubuntu-24.04, x86_64)
  • GitHub Check: build-and-test (windows-2022, x86_64)
  • GitHub Check: upload-dlls (ubuntu-24.04, x86_64)
  • GitHub Check: upload-dlls (macos-14, x86_64)
  • GitHub Check: upload-dlls (windows-2022, x86_64)
🔇 Additional comments (1)
src/polygon_mesh/area_mesh_factory.cpp (1)

187-222: グループ化ロジックの実装が正確です。

combineメソッドにgridMergeと同様のグループベースのアプローチが導入されており、PR目的（LOD1テクスチャ適用のためのgroup単位でのメッシュ分割）を達成しています。

実装の詳細：

  • max_lod_in_objの計算（194-202行）により、各オブジェクトが持つ最大LODを判定
  • highest_lod_onlyオプションのフィルタリング（204-210行）が正しく実装されている
  • group_id = max_lod_in_objの単純化された計算式（215行）は、grid_idが常に0であるため適切

パフォーマンスに関する注意：
以前の実装ではmax_lod_in_objの計算はhighest_lod_onlyが有効な場合のみ実行されていましたが、今回の変更ですべてのオブジェクトに対して実行されるようになりました。これはグループ化に必要な変更ですが、パフォーマンス特性が変わることに留意してください。

Based on learnings

coderabbitai[bot]
coderabbitai bot reviewed Nov 8, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f35536 and 1614f73.

📒 Files selected for processing (1)
  • src/polygon_mesh/area_mesh_factory.cpp (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.941Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。
📚 Learning: 2025-08-18T07:10:22.941Z 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/area_mesh_factory.cpp:195-210
Timestamp: 2025-08-18T07:10:22.941Z
Learning: `area_mesh_factory.cpp`の`gridMerge`と`combine`メソッドでは、`max_lod_in_obj`の計算タイミングが異なり、パフォーマンス特性が微妙に異なる。`gridMerge`では既にグループ分け用に計算済み、`combine`では`highest_lod_only`が有効な場合のみ計算される。

Applied to files:

  • src/polygon_mesh/area_mesh_factory.cpp
📚 Learning: 2025-08-19T07:27:43.111Z 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `area_mesh_factory.cpp`の`gridMerge`では、`group_id`は`grid_id * 100 + max_lod_in_obj`として計算されるため、同一の`group_id`を持つ異なる`grid_id`のペアは存在しない。したがって、`mesh_extractor.cpp`では`group_grid_id.first`のみでノード名を構築しても重複は発生しない。

Applied to files:

  • src/polygon_mesh/area_mesh_factory.cpp
📚 Learning: 2025-08-19T07:27:43.111Z 
Learnt from: sevendev
Repo: Synesthesias/libplateau PR: 281
File: src/polygon_mesh/mesh_extractor.cpp:62-64
Timestamp: 2025-08-19T07:27:43.111Z
Learning: `AreaMeshFactory::gridMerge`では、`group_id = grid_id * (max_lod_in_specification_ + 1) + max_lod_in_obj`の計算式により、各`group_id`は`grid_id`と`max_lod`の組み合わせから一意に決まる。そのため、`mesh_extractor.cpp`の文脈では`group_id`だけでノード名を構築しても重複は発生しない。

Applied to files:

  • src/polygon_mesh/area_mesh_factory.cpp
🧬 Code graph analysis (1)
src/polygon_mesh/area_mesh_factory.cpp (2)
src/polygon_mesh/polygon_mesh_utils.cpp (4)
  • findFirstPolygon (83-97)
  • findFirstPolygon (83-83)
  • getChildCityObjectsRecursive (73-81)
  • getChildCityObjectsRecursive (73-73)
src/polygon_mesh/mesh_extractor.cpp (4)
  • isTypeToSkip (210-217)
  • isTypeToSkip (210-210)
  • shouldContainPrimaryMesh (198-208)
  • shouldContainPrimaryMesh (198-198)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build-and-test (ubuntu-24.04, x86_64)
  • GitHub Check: build-and-test (windows-2022, x86_64)
  • GitHub Check: build-and-test (macos-14, arm64)
  • GitHub Check: check-submodule-license

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@azukizuki